core/txpool/blobpool: extend fully-fetched blob transactions to the full cell set - #35531
Open
cskiraly wants to merge 2 commits into
Open
core/txpool/blobpool: extend fully-fetched blob transactions to the full cell set#35531cskiraly wants to merge 2 commits into
cskiraly wants to merge 2 commits into
Conversation
cskiraly
force-pushed
the
eager-extension
branch
from
August 15, 2026 03:14
d97b518 to
fc31a96
Compare
cskiraly
requested review from
MariusVanDerWijden and
healthykim
and removed request for
gballet and
rjl493456442
August 15, 2026 03:16
Contributor
Author
|
master changed in the meantime, leading to lint catching a changed signature. Need to rebase |
cskiraly
force-pushed
the
eager-extension
branch
from
August 15, 2026 05:30
fc31a96 to
38cc4cd
Compare
…ingest A full fetch downloads only the data cells (columns 0..DataPerBlob-1), stores them, and advertises that mask. The node therefore cannot serve extension columns -- even to samplers whose CL-assigned custody lies in the extension half, who find no provider to fetch from -- and it deviates from the EIP-8070 provider role, which announces full availability. It also cannot act as a full- fetch or availability-check source for other nodes, since both require an all-ones announcement. Complete the cell set locally instead: when the collected cells suffice to reconstruct the blobs but don't cover the extended set, compute the remaining cells in the buffer before the transaction is stored (systematic extension, ~3-4ms/blob -- no KZG erasure solve, and no extra download). Since this happens before the first store write, there is no entry rewrite, no custody change to re-announce, and the advertised all-ones mask is honest: every advertised cell is physically held, so serving involves no on-demand reconstruction a peer could abuse. The reconstructed cells are determined by the already-verified input cells, but the transaction's shipped proofs for the non-custodied indices could not be verified before (their cells did not exist locally). Verify them in one batch (~+6ms/blob) before adopting; a transaction whose own proofs don't match its data is invalid everywhere and is discarded. Sampler (below-DataPerBlob) transactions are stored as delivered, unchanged; the sparse pool's bandwidth savings are unaffected. Providers store the full extended set (~2x the blob payload).
…to end Add an integration test for the headline behavior of the provider extension: a transaction acquired as a full fetch (data cells only) is run through the buffer into a real pool, and must come out fully advertised (GetCustody all-ones, the mask announcements carry) and fully servable (GetBlobCells returns byte-correct extension cells with proofs -- the call behind both the eth GetCells handler and the engine_getBlobsV4 miss path). The test deliberately uses only APIs that predate the provider extension, so it runs unmodified against older code and demonstrates the gap there: advertised custody stays at the 64 data cells and every extension column is returned as nil (verified against master).
cskiraly
force-pushed
the
eager-extension
branch
from
August 19, 2026 13:31
38cc4cd to
b3e2ba5
Compare
| for b := range blobCount { | ||
| for _, idx := range newIndices { | ||
| vcells = append(vcells, all[b*kzg4844.CellsPerBlob+int(idx)]) | ||
| vproofs = append(vproofs, sidecar.Proofs[b*kzg4844.CellProofsPerBlob+int(idx)]) |
Member
There was a problem hiding this comment.
Do we have any preceding check ensuring the full proof set is available? Direct slice access looks a bit dangerous.
Member
There was a problem hiding this comment.
if len(sidecar.Proofs) != len(sidecar.Commitments)*kzg4844.CellProofsPerBlob {
return fmt.Errorf("invalid number of %d proofs compared to %d commitments", len(sidecar.Proofs), len(sidecar.Commitments))
}
OK, we have this validation before accepting the txs into blobPool.
| if n := custody.OneCount(); n >= kzg4844.DataPerBlob && n < kzg4844.CellsPerBlob { | ||
| extended, err := extendCells(sidecar, sorted, custody) | ||
| if err != nil { | ||
| log.Warn("Dropping blob tx with unverifiable extension proofs", "hash", hash, "err", err) |
Member
There was a problem hiding this comment.
Shall we also drop the peer if the transaction contains the invalid Proof? In the extendCells, it will verify the proof with extended cells, if the proof is malformed, we should somehow propagate the error and drop the peer here.
| // not been verified yet, so check them before adopting: a transaction whose | ||
| // own proofs don't match its data is invalid and is discarded. | ||
| if n := custody.OneCount(); n >= kzg4844.DataPerBlob && n < kzg4844.CellsPerBlob { | ||
| extended, err := extendCells(sidecar, sorted, custody) |
Member
There was a problem hiding this comment.
It's an expensive operation with lock held. At least we need to leave a TODO marker, or mitigate the overhead somehow in a following PR.
| log.Warn("Dropping blob tx with unverifiable extension proofs", "hash", hash, "err", err) | ||
| blobBufferExtendFailCounter.Inc(1) | ||
| delete(b.cells, hash) | ||
| delete(b.txs, hash) |
Member
There was a problem hiding this comment.
The transaction is dropped silently after the expensive computation. Isn't it vulnerable to propagate the txs with 64 cells but invalid proofs of the extended cells over and over again?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Built on top of #35529
Extend full blobs received as cells 0-63 eagerly, and store the whole 128 cells. This simplifies checks, and also allows serving getBlobsV4 and peers correctly.
Details:
In the current implementation of sparse blobpool, a full fetch downloads only the 64 data cells and stores/advertises just those. Such a node then can't serve extension columns (64..127) to peers or its own CL, can't act as a full-fetch/availability source for others, and doesn't match the EIP-8070 provider role — even though it holds everything needed to compute any column.
This PR completes the cell set locally at ingest: when the collected, verified cells suffice to reconstruct a blob but don't cover the full set, the pool extends to all 128 cells (systematic reconstruction, no extra download), verifies the transaction's previously-uncheckable extension proofs in one batch (a mismatch means the sidecar is internally inconsistent, so the tx is dropped), and stores it once with all-ones custody. Because this happens before the first store write and first announcement, there's no entry rewrite and no re-announcement, and the advertised availability is honest — serving stays a pure read of physically-held cells.
Sampler transactions (below the reconstruction threshold) are unchanged, preserving the pool's sparsity and bandwidth savings.